-
Notifications
You must be signed in to change notification settings - Fork 1.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: safe http error response #2438
Conversation
Hi. Rather than remove the displaying information from those who want it, can we instead look for a way to avoid having the hook be unable to parse it? Do you have a small repository where you can reproduce the original problem? |
hey @StevenACoffman, thanks for the reply, Initially I tried to control the error with the hooks but to no avail. If it does not persuade you, please point the direction so I could at least try implementing what's requested. |
Hello @koniuszy, maybe another way to solve this would to implement a custom srv.AddTransport(transport.MyCustomPOST{}) |
Signed-off-by: Steve Coffman <steve@khanacademy.org>
Signed-off-by: Steve Coffman <steve@khanacademy.org>
Thanks! So there were actually multiple problems that this uncovered. The main one was that decoding errors were not dispatched, so the hooks had no ability to intercept errors to present the error as you wished. There was also a potential nil pointer dereference which would cause panic and bypass the error presenter hook (although the recover panic would catch that one). I modified your example repository to add a hook. With that change and using this updated PR to gqlgen it will not present the details you wanted to be obscured:
|
I cannot intercept the error of an invalid request using the hooks. I always get:
This change removes from the response the details of a decoding error, and instead returns a StatusBadRequest with a more generic decoding error. The more specific error is instead logged.
Before:
After:
I have:
Solved:
#2430